Skip to content

fix(acp): forward resolved credential pool#70293

Open
stefanpieter wants to merge 1 commit into
NousResearch:mainfrom
stefanpieter:fix/acp-forward-credential-pool
Open

fix(acp): forward resolved credential pool#70293
stefanpieter wants to merge 1 commit into
NousResearch:mainfrom
stefanpieter:fix/acp-forward-credential-pool

Conversation

@stefanpieter

@stefanpieter stefanpieter commented Jul 23, 2026

Copy link
Copy Markdown

What does this PR do?

ACP resolved complete provider runtime state but dropped the selected credential_pool when constructing AIAgent. Long-lived OpenAI Codex ACP sessions therefore could not use Hermes' existing in-process OAuth 401 refresh/rotation path and required an ACP restart after token expiry.

This forwards the already resolved, provider-scoped pool into ACP-created agents. It keeps authentication ownership in Hermes core, preserves existing account/provider guards, and requires no VS Code extension-side token handling or routine ACP replacement.

Related Issue

Fixes #70292

Related to #70097 / #70111, but distinct: those address refresh/adoption once a credential pool is attached; this fixes ACP omitting the pool entirely.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • acp_adapter/session.py: forward runtime["credential_pool"] into ACP-created AIAgent instances.
  • tests/acp/test_session.py: extend the ACP construction regression to assert that resolved credential pools are retained.

How to Test

  1. Configure openai-codex OAuth and start a long-lived hermes acp process.
  2. Let or simulate the active access token expiring while current token state remains refreshable/adoptable through Hermes' credential pool.
  3. Send another prompt through the same ACP process and verify it refreshes/rotates and retries without terminating ACP.

Automated/local verification:

HERMES_TEST_FILE_RETRIES=0 scripts/run_tests.sh tests/acp/ tests/acp_adapter/ tests/run_agent/test_run_agent_codex_responses.py
438 passed, 0 failed

A real patched ACP JSON-RPC smoke request returned exactly ACP_POOL_OK in the same ACP PID; the temporary session was deleted. A live runtime invariant check also confirmed that the selected pool entry matches the agent API key and that the pool provider matches the agent provider.

Full-suite baseline comparison:

  • The complete scripts/run_tests.sh run reported 28 failures in 17 files outside the changed ACP/Codex scope.
  • Running those exact 17 files against the untouched base commit c4f5a45d reproduced the same 28 failures.
  • Neither the branch nor baseline failures include acp_adapter/session.py, tests/acp/, tests/acp_adapter/, or the Codex-response recovery suite.

Checklist

Code

  • I've read the Contributing Guide and repository AGENTS.md.
  • My commit messages follow Conventional Commits.
  • I searched for existing PRs to make sure this isn't a duplicate.
  • My PR contains only changes related to this fix.
  • I've run the repository's required scripts/run_tests.sh CI-parity wrapper.
  • I've added tests for my changes.
  • I've tested on macOS.

Documentation & Housekeeping

  • Documentation update: N/A; no user-facing configuration or command changes.
  • cli-config.yaml.example: N/A; no config keys changed.
  • CONTRIBUTING.md / AGENTS.md: N/A; no architecture or workflow change.
  • Cross-platform impact considered: the change forwards an existing Python object and is platform-independent.
  • Tool descriptions/schemas: N/A; no tool behavior changed.

Screenshots / Logs

same_acp_pid: 20791
response: ACP_POOL_OK
stopReason: end_turn
stderr_error_lines: []

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/acp Agent Communication Protocol adapter provider/openai OpenAI / Codex Responses API area/auth Authentication, OAuth, credential pools sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 23, 2026
@joelbrilliant

Copy link
Copy Markdown
Contributor

Cross-link for maintainers: we hit the same ACP missing-credential_pool bug independently (long-lived ACP host + OAuth).

Your PR (#70293) is the clean minimal fix for pool handoff — thanks for landing that path.

We also opened #70465 with the same handoff plus 403-shaped OAuth recovery (xAI often returns bad-credentials as HTTP 403, not 401). Happy for #70293 to merge first; we will rebase #70465 and drop the duplicated line if so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/acp Agent Communication Protocol adapter P2 Medium — degraded but workaround exists provider/openai OpenAI / Codex Responses API sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACP drops credential pool, requiring restart after OAuth token expiry

3 participants